fix(client): align device preference persistence with permission and track end events#2196
Merged
fix(client): align device preference persistence with permission and track end events#2196
Conversation
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughDevice managers now gate applying and persisting device preferences on browser permission state, only applying or storing preferences when the browser permission observable reports Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as "App / DeviceManager"
participant Perm as "BrowserPermission\n(asStateObservable / firstValueFrom)"
participant Devices as "Media Devices\n(select/enable)"
participant Store as "Preference Store"
App->>Perm: request current permission state
alt permission === 'granted'
Perm-->>App: 'granted'
App->>Store: read persisted preferences
Store-->>App: persisted prefs
App->>Devices: apply persisted preferences (select/enable)
Devices-->>App: device ready
App->>Store: persist new selection (on changes)
else permission != 'granted'
Perm-->>App: 'prompt' / 'denied'
App->>Devices: enable defaults (do not apply persisted prefs)
Devices-->>App: device ready
Note right of App: skip writing persisted prefs
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/client/src/devices/DeviceManager.ts`:
- Around line 92-103: The guard in the combineLatest callback currently uses the
broad boolean this.isTrackStoppedDueToTrackEnd which blocks all persistence
writes during the flag window; change the logic in the combineLatest
subscription (the callback that receives [selectedDevice, status,
browserPermissionState]) to stop ignoring writes only when the persisted state
truly matches a track-end stop rather than any change: remove the global
isTrackStoppedDueToTrackEnd check from the unconditional early return and
instead compare the current selectedDevice (from selectedDevice$) against the
priorSelectedDevice (store previous value in a local variable inside
DeviceManager) and only skip persistence when priorSelectedDevice indicates it
was stopped due to track end and the new selectedDevice is identical (i.e., no
user-initiated recovery); keep the browserPermissionState and status checks
as-is.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e0f4a120-0a8c-4beb-ba94-48bcd330bece
📒 Files selected for processing (8)
packages/client/src/devices/CameraManager.tspackages/client/src/devices/DeviceManager.tspackages/client/src/devices/MicrophoneManager.tspackages/client/src/devices/SpeakerManager.tspackages/client/src/devices/__tests__/CameraManager.test.tspackages/client/src/devices/__tests__/DeviceManager.test.tspackages/client/src/devices/__tests__/MicrophoneManager.test.tspackages/client/src/devices/__tests__/SpeakerManager.test.ts
oliverlaz
approved these changes
Apr 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
💡 Overview
This PR fixes a bug where device preferences were applied when the browser had not yet granted permissions, which could trigger unexpected permission prompts. Now applyPersistedPreferences is gated behind a
browserPermissionState === 'granted'check, so preferences are only restored when the user has already granted access. Additionally, when a track ends unexpectedly (tab close, device disconnect), we no longer store that state to device preferences, preventing incorrect muted/disabled preferences from being persisted.📝 Implementation notes
🎫 Ticket: https://linear.app/stream/issue/REACT-944/align-device-preferences-with-browser-permissions
📑 Docs: https://github.com/GetStream/docs-content/pull/
Summary by CodeRabbit